Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use const generics in graph module #152

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

chaosprint
Copy link

The graph module uses a fixed length of 64, which limits its usage in many contexts such as web audio.

#![feature(min_const_generics)] can solve this issue, and will become stable in Rust 1.51
rust-lang/rust#79135

Currently tested successful and can be compiled using nightly version of rustc.

@chaosprint
Copy link
Author

Hi @mitchmindtree @est31 can we get this merged?

@mitchmindtree
Copy link
Member

Thanks for the PR @chaosprint!

I do agree there could be a case for tweaking this value, though more for tweaking the latency-vs-performance trade-off. Would you mind elaborating on how this change helps to enable use with webaudio or other contexts? Note that, if the audio backend provides buffers of some other size, you can just request audio from the graph multiple times per system audio callback for example.

Is there any chance we can use default values for the const generics here to retain the original behaviour by default and not break the API? Or are they not available with const generics yet?

@chaosprint
Copy link
Author

chaosprint commented Mar 26, 2021

Thanks for the PR @chaosprint!

Hi, thanks for the reply.

I do agree there could be a case for tweaking this value, though more for tweaking the latency-vs-performance trade-off. Would you mind elaborating on how this change helps to enable use with webaudio or other contexts? Note that, if the audio backend provides buffers of some other size, you can just request audio from the graph multiple times per system audio callback for example.

I am currently using dasp_graph for my project Glicol, a graph-oriented live coding language.

The idea is to convert the code string directly to an audio graph. The glicol.rs is compiled to a wasm file and runs in the browser.

Previously, I used the default 64 block size in dasp_graph and called the process method twice to fill the 128 block size of web audio. The first prominent disadvantage was that there would be a repeated block of code. And after I forked the repo and used the min const generics feature from the nightly Rust, I noticed that in the Web Audio tool in Chrome, the CPU usage significantly decreases, around 1.2-1.5 times.

Besides, I have been running Glicol on Bela, where 64 may be too large to get the full potential of Bela low-latency design.

Is there any chance we can use default values for the const generics here to retain the original behaviour by default and not break the API? Or are they not available with const generics yet?

Thus, I think that it may be hard to find a proper default value for block size, at least for the two cases I mentioned above. Previously I also tried to clone and change the value locally or forked it to different branches but I feel it is not so ergonomic and it may not work for some cases. For example, if a user wishes to create a modular synth project using dasp_graph, it might be good to be able to change the block size in the app (pls correct me if I am wrong here). I am not sure if there is any other way to do it, but as the const generics feature comes to the stable version, I think this graph module might be a good use case for the feature, and users of this library may also be happy and capable enough to tweak the block size.

I also think that having the block size specified clear in trait implementation can be helpful for avoiding some errors, especially when we may have different block sizes in different projects.

impl Node<64> for MyNode {
    fn process(&mut self, inputs: &[Input<64>], output: &mut [Buffer<64>]) {
        // sometimes we may use a for loop here
    }
}

@est31
Copy link
Member

est31 commented Mar 26, 2021

Is there any chance we can use default values for the const generics here to retain the original behaviour by default and not break the API

From reading #133 it seems the new release is going to be 0.12, so API breakages should be ok? Am I missing something?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants